Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code refactoring (No functional changes) #7513

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

atiq-bs23
Copy link
Contributor

@atiq-bs23 atiq-bs23 commented Jan 24, 2025

  • Removed unused variables.
  • Included reference in ascending order.
  • Adjusted spaces/alignments.

@RomanovM
Copy link
Contributor

@atiq-bs23 Thanks for your help. How did you find all these places, manually or by some code analysis utilities?

@RomanovM RomanovM self-assigned this Jan 24, 2025
@danFbach
Copy link
Contributor

@RomanovM Have you thought about using a github action workflow to run linting check on new PRs so that this is taken care of automatically?

@RomanovM
Copy link
Contributor

@danFbach Yep #5538

@danFbach
Copy link
Contributor

Okay 👍

@atiq-bs23
Copy link
Contributor Author

@RomanovM yes, using visual studio code cleanup.

@atiq-bs23
Copy link
Contributor Author

@RomanovM
I have some previous development experience with you, and I know that nopCommerce follows best practices.
While exploring your recent changes, I ran Visual Studio's default Code Analysis and Code Cleanup.

This process affected many files, mostly by changing their size. However, I noticed a few files that had meaningful changes, such as adjustments to spacing and alignment, sorting using references in ascending order, and removing unused references.

I included only these meaningful changes in the commit.

image

@RomanovM RomanovM merged commit 69e2062 into nopSolutions:develop Jan 27, 2025
@atiq-bs23 atiq-bs23 deleted the code-refactoring branch January 27, 2025 07:15
@atiq-bs23
Copy link
Contributor Author

@RomanovM While manually checking the changed files, I missed a few of them. I’ve now found the ones I missed. Should I include those in a pull request?

@RomanovM
Copy link
Contributor

@atiq-bs23 You can create a new pull request

@atiq-bs23
Copy link
Contributor Author

@RomanovM #7522

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants